Skip to content

Extract TitleBuilder for PR title composition#14285

Open
kbukum1 wants to merge 3 commits intomainfrom
kbukum1/refactor-pr-gen-components
Open

Extract TitleBuilder for PR title composition#14285
kbukum1 wants to merge 3 commits intomainfrom
kbukum1/refactor-pr-gen-components

Conversation

@kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Feb 25, 2026 •
edited
Loading

What are you trying to accomplish?

Extract a lightweight TitleBuilder class from MessageBuilder so that PR title composition (prefix + capitalization + base title) can be reused by dependabot-api for multi-ecosystem combined PRs.

The current MessageBuilder inlines prefix/capitalize logic in pr_name, which cannot be called from dependabot-api without instantiating the entire builder. The API currently hardcodes combined PR titles, completely bypassing commit-message prefix configuration — causing #14032 and #10057.

Fixes #14032
Fixes #10057
Related: #12178

What changed?

1 new file — common/lib/dependabot/pull_request_creator/message_builder/title_builder.rb:

  • TitleBuilder#build — composes prefix + capitalization + base title
  • Works with a full PrNamePrefixer (updater path) or lightweight commit_message_options hash (API path, no network calls)
  • TitleBuilder.multi_ecosystem_base_title — class method for generating multi-ecosystem group titles

MessageBuilder#pr_name now delegates to TitleBuilder instead of inlining the prefix/capitalize logic. No feature flag — the change is a pure extraction with identical behavior.

Anything you want to highlight for special attention from reviewers?

The change to MessageBuilder is minimal (5 lines changed). TitleBuilder follows the same pattern as existing helpers (IssueLinker, MetadataPresenter, LinkAndMentionSanitizer).

The companion dependabot-api PR (github/dependabot-api#7792) uses TitleBuilder to fix the multi-ecosystem prefix bug.

How will you know you have accomplished your goal?

  • New TitleBuilder spec passes (10 examples covering prefix, capitalization, scope, dev dependencies, multi-ecosystem)
  • Existing MessageBuilder specs continue to pass (behavior unchanged)
  • dependabot-api companion PR can use TitleBuilder to apply commit-message prefix to combined PRs

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@kbukum1 kbukum1 force-pushed the kbukum1/refactor-pr-gen-components branch 4 times, most recently from 7b980a2 to bd3e3b1 Compare March 4, 2026 07:54
@kbukum1 kbukum1 changed the title Refactor: Extract TitleBuilder and strategy components from MessageBuilder Extract TitleBuilder from MessageBuilder for reusable PR title composition Mar 4, 2026
@kbukum1 kbukum1 changed the title Extract TitleBuilder from MessageBuilder for reusable PR title composition Extract TitleBuilder for PR title composition Mar 4, 2026
@kbukum1 kbukum1 force-pushed the kbukum1/refactor-pr-gen-components branch 2 times, most recently from 9cc56cb to c90bf8f Compare March 4, 2026 19:01
@kbukum1 kbukum1 marked this pull request as ready for review March 4, 2026 19:01
@kbukum1 kbukum1 requested a review from a team as a code owner March 4, 2026 19:01
Copilot AI review requested due to automatic review settings March 4, 2026 19:01
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extracts PR title composition logic out of MessageBuilder into a new TitleBuilder helper so title prefixing/capitalization can be reused (notably by dependabot-api for multi-ecosystem combined PRs) while keeping MessageBuilder#pr_name behavior consistent.

Changes:

  • Add TitleBuilder to compose a final PR title from a base title plus either a PrNamePrefixer (updater path) or explicit commit_message_options (API path).
  • Update MessageBuilder#pr_name to delegate title composition to TitleBuilder.
  • Add specs covering prefix, scope, dev prefix behavior, and multi-ecosystem base title generation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
common/lib/dependabot/pull_request_creator/message_builder/title_builder.rb Introduces extracted title composition logic intended for reuse outside MessageBuilder.
common/lib/dependabot/pull_request_creator/message_builder.rb Delegates pr_name formatting to TitleBuilder and requires the new helper.
common/spec/dependabot/pull_request_creator/message_builder/title_builder_spec.rb Adds unit tests for the new TitleBuilder behavior.

You can also share your feedback on Copilot code review. Take the survey.

@kbukum1 kbukum1 force-pushed the kbukum1/refactor-pr-gen-components branch from c90bf8f to c44f7c3 Compare March 4, 2026 19:21
@kbukum1 kbukum1 requested a review from Copilot March 4, 2026 20:50
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

You can also share your feedback on Copilot code review. Take the survey.

@kbukum1 kbukum1 requested a review from Copilot March 4, 2026 21:04
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

You can also share your feedback on Copilot code review. Take the survey.

common/spec/dependabot/pull_request_creator/message_builder/title_builder_spec.rb
Comment on lines +5 to +20
require "dependabot/pull_request_creator/pr_name_prefixer"
require "dependabot/pull_request_creator/message_builder/title_builder"

RSpec.describe Dependabot::PullRequestCreator::MessageBuilder::TitleBuilder do
before do
Dependabot::Dependency.register_production_check(
"npm_and_yarn",
lambda do |groups|
return true if groups.empty?
return true if groups.include?("optionalDependencies")

groups.include?("dependencies")
end
)
end

Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spec registers a global production check for npm_and_yarn, which mutates Dependabot::Dependency state for the rest of the test suite (order-dependent) and duplicates the canonical lambda already defined in npm_and_yarn/lib/dependabot/npm_and_yarn.rb. Prefer requiring dependabot/npm_and_yarn to use the real registration, or avoid global registration by stubbing Dependency#production? / using test doubles for dependencies in these examples.

Suggested change
require "dependabot/pull_request_creator/pr_name_prefixer"
require "dependabot/pull_request_creator/message_builder/title_builder"
RSpec.describe Dependabot::PullRequestCreator::MessageBuilder::TitleBuilder do
before do
Dependabot::Dependency.register_production_check(
"npm_and_yarn",
lambda do |groups|
return true if groups.empty?
return true if groups.include?("optionalDependencies")
groups.include?("dependencies")
end
)
end
require "dependabot/npm_and_yarn"
require "dependabot/pull_request_creator/pr_name_prefixer"
require "dependabot/pull_request_creator/message_builder/title_builder"
RSpec.describe Dependabot::PullRequestCreator::MessageBuilder::TitleBuilder do

Copilot uses AI. Check for mistakes.
common/lib/dependabot/pull_request_creator/message_builder/title_builder.rb

sig { returns(T::Boolean) }
def production_dependencies?
dependencies&.any?(&:production?) != false
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

production_dependencies? treats an empty dependencies array as non-production because [].any? is false (so the method returns false). Given dependencies is optional on this helper, callers may reasonably pass [] to mean “unknown/no deps”; in that case we likely want to default to production (or at least keep behavior consistent with nil, which currently defaults to production). Consider changing this to treat nil/empty as production (e.g., return true unless dependencies&.all? { |d| !d.production? }).

Suggested change
dependencies&.any?(&:production?) != false
return true if dependencies.nil? || dependencies.empty?
dependencies.any?(&:production?)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@yeikel yeikel yeikel left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Commit message prefix ignored Prefix message not working on commit and PR

3 participants

@kbukum1 @yeikel